-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement applied constructor types #22543
base: main
Are you sure you want to change the base?
Conversation
a12b589
to
f64cc82
Compare
The CI failure seems to be the same as on |
Co-authored-by: Kacper Korban <[email protected]>
- only type applied constructor types in Typer - handle multiple parameter lists - add tests for mixed tracked and not tracked parameters
examples; Hide more implementation parts under modularity
51894ab
to
64aee9d
Compare
I've noticed during your presentation that it also works for import language.experimental.modularity
class Box[T](using tracked val value: Int)
def main =
given Int = 42
val b: Box() = Box() after typer: def main: Unit =
final lazy given val given_Int: Int = 42
val b: Box[Any]{val value: (given_Int : Int)} = new Box[Any](given_Int)() |
@@ -2018,7 +2008,7 @@ object Parsers { | |||
/** SimpleType ::= SimpleLiteral | |||
* | ‘?’ TypeBounds | |||
* | SimpleType1 | |||
* | SimpleType ‘(’ Singletons ‘)’ -- under language.experimental.dependent, checked in Typer | |||
* | SimpleType ‘(’ Singletons ‘)’ -- under language.experimental.modularity, checked in Typer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked in Typer
is no longer correct, we do check the feature flag here.
@@ -1694,6 +1694,23 @@ trait Applications extends Compatibility { | |||
def typedUnApply(tree: untpd.UnApply, selType: Type)(using Context): UnApply = | |||
throw new UnsupportedOperationException("cannot type check an UnApply node") | |||
|
|||
def typedAppliedConstructorType(tree: untpd.Apply)(using Context) = untpd.methPart(tree) match | |||
case Select(New(tpt), _) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 4 spaces of indent here?
@@ -1694,6 +1694,23 @@ trait Applications extends Compatibility { | |||
def typedUnApply(tree: untpd.UnApply, selType: Type)(using Context): UnApply = | |||
throw new UnsupportedOperationException("cannot type check an UnApply node") | |||
|
|||
def typedAppliedConstructorType(tree: untpd.Apply)(using Context) = untpd.methPart(tree) match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add a doc comment explaining what kind of expressions this typechecks.
def typedAppliedConstructorType(tree: untpd.Apply)(using Context) = untpd.methPart(tree) match | ||
case Select(New(tpt), _) => | ||
val tree1 = typedExpr(tree) | ||
val widenSkolemsMap = new TypeMap: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
widenSkolemsMap is defined in the same way in NamerOps. I suggest we move the whole thing to TypeUtils, into an extension method widenSkolems
and refer to that from both places.
val preciseTp = widenSkolemsMap(tree1.tpe) | ||
val classTp = typedType(tpt).tpe | ||
def classSymbolHasOnlyTrackedParameters = | ||
classTp.classSymbol.primaryConstructor.paramSymss.flatten.filter(_.isTerm).forall(_.is(Tracked)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
classTp.classSymbol.primaryConstructor.paramSymss.flatten.filter(_.isTerm).forall(_.is(Tracked)) | |
!classTp.classSymbol.primaryConstructor.paramSymss.nestedExists: param => | |
param.isTerm && !param.is(Tracked) |
This avoids the costly flatten.
classTp.classSymbol.primaryConstructor.paramSymss.flatten.filter(_.isTerm).forall(_.is(Tracked)) | ||
if !preciseTp.isError && !classSymbolHasOnlyTrackedParameters then | ||
report.warning(OnlyFullyDependentAppliedConstructorType(), tree.srcPos) | ||
if !preciseTp.isError && (preciseTp frozen_=:= classTp) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not understand the condition. I guess you want to exclude constructors with no value parameters, but I am not clear why the condition preciseTp frozen_=:= classTp
is equivalent to that.
@@ -3485,7 +3477,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||
|
|||
/** Typecheck tree without adapting it, returning a typed tree. | |||
* @param initTree the untyped tree | |||
* @param pt the expected result type | |||
* @param pt the expected result typ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo here?
@@ -3525,7 +3517,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||
|
|||
def typedUnnamed(tree: untpd.Tree): Tree = tree match { | |||
case tree: untpd.Apply => | |||
if (ctx.mode is Mode.Pattern) typedUnApply(tree, pt) else typedApply(tree, pt) | |||
if (ctx.mode is Mode.Pattern) typedUnApply(tree, pt) | |||
else if (Feature.enabled(modularity) && ctx.mode.is(Mode.Type) && !ctx.isAfterTyper) typedAppliedConstructorType(tree) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new case should not be added here. This is about the hottest code that exists in the compiler. We need to keep it as small as possible. Instead I would suggest to add the distinction to Applications.scala following line 1283:
val app = tree.fun match
case Select(New(tpt), _)
if ctx.mode.is(Mode.Type) && Feature.enabled(modularity && !ctx.isAfterTyper =>
typedAppliedConstructorType(tpt, tree.args)
case untpd.TypeApply(_: untpd.SplicePattern, _) if Feature.quotedPatternsWithPolymorphicFunctionsEnabled =>
...
report.warning(PointlessAppliedConstructorType(tpt, tree.args, classTp), tree.srcPos) | ||
TypeTree(preciseTp) | ||
case _ => | ||
throw TypeError(em"Unexpected applied constructor type: $tree") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this ever happen for a parsed program? If not we should throw an assertion instead of a TypeError. But with the refactoring proposed in Typer, we can avoid the pattern match here and hence the error case would be eliminated.
+++ | SimpleType1 {ParArgumentExprs} | ||
``` | ||
|
||
A `SimpleType` can now optionally be followed by `ParArgumentExprs`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add some explanations how argument expressions map to refinements.
closes #22542
Introduce new syntax to make classes with
tracked
parameterseasier to use. The new syntax is essentially the ability to use an application
of a class constructor as a type, we call such types applied constructor types.
With this new feature the following example compiles correctly and the type in
the comment is the resulting type of the applied constructor types.
Syntax change
A
SimpleType
can now optionally be followed byParArgumentExprs
.